Skip to content

Conversation

Byron
Copy link
Collaborator

@Byron Byron commented Oct 6, 2025

Get a minimal viable version of apply() that only deals with the common case we can handle today, or simple cases that are new.

Follow-up of #10447.

Tasks

  • but-testing apply for ease of use in the real-world
  • test were we apply an unnamed segment (i.e. ambiguous) and reproduce the issue of 'last ditch failure to add' which ends up with changed merge commit somehow.
  • all the missing tests for WorkspaceCommit::merge(), but keep it minimal
    • implement 'force' as well

Warning

Current implementation seems does very strange things in conjunction with the 'old' unapply, workspace commit ends up in a strange state and it generally doesn't work as expected.

Next PR?

  • unapply: make sure to also delete metadata, even if the branch isn't actually in the workspace
    • remember: ghost-stacks when the stack is in the workspace metadata, but not reachable from the workspace commit.
  • cherry-pick index as well (but only conflicts) - consider skipping but make sure it doesn't get lost

Future Tasks

  • proper worktree handling - we must not checkout branches that are already checked out in worktree (see gix code somewhere)
  • Permutations: starting point
    • test unborn
    • starting point without WS ref
    • starting point with WS ref but not WS-commit or metadata
    • starting point with WS ref and WS commit
  • Permutations: base position
    • base below
    • base above
  • Validate StackID handling in VB.toml adapter (assure it can keep unapplied branches correctly)
    • We need all the tests so at a later point we can possibly localise the stack-id and keep it with each branch instead.
  • without single-branch mode, it's possible to have a workspace with just one stack, or even no stacks at all.
  • unapply: must take care of assignments (see research)
  • don't apply the target branch!
  • try to fix but-graph issue

Shortcomings

  • Worktree change in a detached HEAD can't be stashed as there is no reference to associated the stash with.
    • But that's OK as we don't currently store stashes anyway for a lack of UI support to try to apply them.

Notes

General Rules

  • The workspace is a conflict-free zone
    • nothing that operates on the conflict must write conflicts into the index.
      This is as conflicts are currently hidden from view.
  • Symmetry
    • If apply is doing something, then unapply undoes exactly that, or in other words State + apply + unapply == State
  • There is no single-branch workspace when starting in single-branch mode
    • A workspace consists of at least two branches and a workspace commit
    • The workspace commit is optional if there is only one commit involved, i.e. when it's just a bunch of branches on top of a single commit
  • Workspace Commit ALWAYS for even for a single branch
    • The workspace backend can deal with anything, but commit() currently can't.
    • Have to add commit() and uncommit() as well to all apply-unapply tests so these can later be re-tested with different behaviour.
    • Implied by the previous rule

Follow-Ups

  • commit with auto-workspace-commit creation
    • At the same time, it needs uncommit() that is symmetric

Thus:

  • snapshots of worktree changes will be made to apply by forcing merge-conflicts to be... auto-resolved.
    This is a problem, but we can't have conflicts as the UI doesn't show them right now, nor does it allow interacting with them.

Unapply

  • Conflicting paths are passed added as extra commit at first, without additional special handling in apply just to be able to handle them.
    • This means assignments aren't taken care of in all cases (but we will see how all this interacts with stack-ids)

Research

Unapply: Assignments - with stashing

  • uncommitted but assigned changes should create a snapshot commit
  • when applying the same branch this snapshot is applied

However, the user should be able to interact with these.

Unapply: Assignments - with WIP commit

  • uncommitted but assigned changes should create a WIP commit
    • or just unassign these assignments and they are back in the unassigned changes of the workspace
  • MVP apply: do nothing with the WIP commit
  • final version: apply restores the assignments from the WIP commit (which then is tracked with metadata)

Unapply with worktree changes

  • worktree changes that don't re-apply cleanly

Possible Follow-Ups

  • Find a way to display and handle conflicts in the UI (Gitizen).
    • this would allow us to write conflicts as well and deal with them.

Copy link

vercel bot commented Oct 6, 2025

@Byron is attempting to deploy a commit to the GitButler Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions bot added the rust Pull requests that update Rust code label Oct 6, 2025
@Byron Byron force-pushed the next branch 2 times, most recently from 954f74d to bf8d145 Compare October 6, 2025 15:18
Otherwise, we think we applied the branch, but it's not enough
due to ambiguity. Then the stack or segment wouldn't actually
show up unless we make the right choice of adding it as dependent
branch instead of stack.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants